Skip to content

Add fetch metadata module - #2271

Open
sdelamo wants to merge 19 commits into
5.4.xfrom
fetch-metadata
Open

Add fetch metadata module#2271
sdelamo wants to merge 19 commits into
5.4.xfrom
fetch-metadata

Conversation

@sdelamo

@sdelamo sdelamo commented Aug 21, 2026

Copy link
Copy Markdown
Member

Copilot AI lite review requested due to automatic review settings August 21, 2026 13:54
@sdelamo sdelamo added the type: enhancement New feature or request label Aug 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the security-fetch-metadata module for resource-isolation filtering, configurable rules, CORS handling, tests, documentation, and Java/Kotlin/Groovy examples. Two unresolved CORS issues remain: server-only bean gating and correct OPTIONS preflight method handling.

Changes:

  • Adds Fetch Metadata APIs, filters, built-in rules, configuration, and tests.
  • Adds documentation and multilingual examples.
  • Registers the module and updates build metadata.
  • Requires fixes to CORS bean conditions and preflight evaluation.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.

Show a summary per file
File Summary
test-suite/src/test/java/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRuleTest.java Java example test
test-suite/src/test/java/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRule.java Java custom-rule example
test-suite/build.gradle Test suite dependency
test-suite-kotlin/src/test/kotlin/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRuleTest.kt Kotlin example test
test-suite-kotlin/src/test/kotlin/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRule.kt Kotlin custom-rule example
test-suite-kotlin/build.gradle.kts Kotlin test suite dependency
test-suite-groovy/src/test/groovy/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRuleSpec.groovy Groovy example specification
test-suite-groovy/src/test/groovy/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRule.groovy Groovy custom-rule example
test-suite-groovy/build.gradle Groovy test suite dependency
src/main/docs/guide/toc.yml Guide navigation
src/main/docs/guide/fetchMetadata/fetchMetadataResourceIsolationPolicy.adoc Resource-isolation policy documentation
src/main/docs/guide/fetchMetadata/fetchMetadataInstallation.adoc Installation documentation
src/main/docs/guide/fetchMetadata/fetchMetadataCustomRules.adoc Custom-rule documentation
src/main/docs/guide/fetchMetadata/fetchMetadataCors.adoc CORS documentation
src/main/docs/guide/fetchMetadata/fetchMetadataConfiguration.adoc Configuration documentation
src/main/docs/guide/fetchMetadata.adoc Module overview
settings.gradle Module registration
security-fetch-metadata/src/test/resources/logback.xml Test logging configuration
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/rules/FetchMetadataRulesConfigurationPropertiesTest.java Rule configuration tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/rules/CrossOriginFetchMetadataRuleTest.java CORS matching tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/rules/BuiltInFetchMetadataRulesTest.java Built-in rule tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/rules/BuiltInFetchMetadataRuleOrderTest.java Rule ordering tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/NoFetchMetadataDefaultTest.java Missing-metadata default tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/FetchMetadataTestController.java Functional test controller
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterTest.java Filter decision tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterPatternTest.java Path filtering tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterFunctionalTest.java End-to-end filter tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterDisabledTest.java Disabled-filter tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterConfigurationPropertiesTest.java Filter configuration tests
security-fetch-metadata/src/test/java/io/micronaut/security/fetchmetadata/CrossOriginFetchMetadataRuleDisabledTest.java Disabled CORS exemption tests
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/SimpleTopLevelNavigationAndIframingFetchMetadataRule.java Navigation and iframing rule
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/SameSiteFetchMetadataRule.java Same-site rule
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/SameOriginFetchMetadataRule.java Same-origin rule
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/package-info.java Rules package documentation
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/NoFetchMetadataRule.java Missing-metadata rule
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/FetchMetadataRulesConfigurationProperties.java Rule configuration properties
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/FetchMetadataRulesConfiguration.java Rule configuration API
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/FetchMetadataRuleOrder.java Built-in rule ordering
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/CrossOriginFetchMetadataRule.java CORS exemption rule; server gating and preflight handling require fixes
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/BrowserInitiatedRequestFetchMetadataRule.java Browser-initiated request rule
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/package-info.java Package documentation
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/HttpRequestFetchMetadataRule.java HTTP request rule API
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/FetchMetadataRuleResult.java Rule result model
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/FetchMetadataRule.java Generic rule API
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterConfigurationProperties.java Filter configuration properties
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/FetchMetadataFilterConfiguration.java Filter configuration API
security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/FetchMetadataFilter.java Server filter
security-fetch-metadata/build.gradle.kts Module build configuration
gradle/libs.versions.toml Dependency version metadata

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

sdelamo and others added 5 commits August 21, 2026 16:15
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Validate preflight requests against the method declared by Access-Control-Request-Method instead of OPTIONS, while retaining the request method as a fallback.

Add unit and functional coverage for allowed and rejected preflight methods.
@sdelamo
sdelamo requested review from yawkat and a lite review from Copilot August 21, 2026 16:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.

Suppressed comments (4)

security-fetch-metadata/src/main/java/io/micronaut/security/fetchmetadata/rules/CrossOriginFetchMetadataRule.java:64

  • This package is @NullMarked, but orElse(null) makes origin nullable. Declaring it as non-null can trigger the repository's JSpecify/nullness checks and misstates the value's contract; annotate the local as @Nullable (or keep the value in an Optional) before checking it for null.
        String origin = request.getOrigin().orElse(null);

test-suite-groovy/src/test/groovy/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRule.groovy:5

  • The snippet::... [tags="imports,clazz"] used by the guide excludes this Requires import because the imports tag starts on the following line. The generated Groovy example therefore contains an unresolved @Requires annotation; start the tag before this import (or move the import inside the tagged block).
    test-suite-kotlin/src/test/kotlin/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRule.kt:5
  • The snippet::... [tags="imports,clazz"] used by the guide excludes this Requires import because the imports tag starts on the following line. The generated Kotlin example therefore contains an unresolved @Requires annotation; start the tag before this import (or move the import inside the tagged block).
    test-suite/src/test/java/io/micronaut/security/docs/fetchmetadata/PublicResourceFetchMetadataRule.java:5
  • The snippet::... [tags="imports,clazz"] used by the guide excludes this Requires import because the imports tag starts on the following line. The generated Java example therefore contains an unresolved @Requires annotation; start the tag before this import (or move the import inside the tagged block).

*/
@Requires(property = FetchMetadataFilterConfigurationProperties.PROPERTY_ENABLED, value = StringUtils.TRUE, defaultValue = StringUtils.TRUE)
@Requires(classes = ServerFilter.class)
@ServerFilter("${" + FetchMetadataFilterConfigurationProperties.PREFIX + ".pattern:" + ServerFilter.MATCH_ALL_PATTERN + "}")
Comment on lines +46 to +48
if (secFetch.mode() == Mode.NAVIGATE &&
request.getMethod() == HttpMethod.GET &&
(secFetch.dest() != Destination.OBJECT && secFetch.dest() != Destination.EMBED)
@sdelamo
sdelamo requested a review from yawkat August 24, 2026 07:35
@sdelamo
sdelamo requested a review from dstepanov September 2, 2026 13:47

@dstepanov dstepanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice module — clean Ordered rule SPI, per-rule toggles, defaults that track the web.dev reference policy, and honest docs about the limitations. Functional coverage is good too.

Left inline comments. The two I'd most like to see addressed before merge are the fail-open on unrecognized Sec-Fetch-* values (NoFetchMetadataRule) and the missing toggle on SimpleTopLevelNavigationAndIframingFetchMetadataRule. The rest are smaller.

One thing I checked and it's fine: @since 5.4.0 and binaryCompatibility.enabledAfter("5.4.0") both line up with projectVersion=5.4.0-SNAPSHOT.


@Override
public FetchMetadataRuleResult check(HttpRequest<?> request, @Nullable SecFetch secFetch) {
if (secFetch == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail-open on unrecognized values, not just absent ones.

SecFetch.of() returns null if any of Sec-Fetch-Site / -Mode / -Dest carries a value not in the corresponding enum (and also when Sec-Fetch-User is present with a value other than ?1). Since this rule runs first (order 100) and is enabled by default, a real browser request carrying a destination micronaut-http doesn't know is fully exempted from the policy instead of being evaluated.

That isn't hypothetical: io.micronaut.http.Destination currently has no webidentity (FedCM), dictionary, webbundle, or sharedstorageworklet, and the Fetch spec's destination list only grows. Every future browser addition silently widens the hole, and nothing here will fail loudly when it happens.

Sec-Fetch-Site alone is the signal for "client is not Fetch-Metadata-aware". Suggest keying on its absence directly:

if (!request.getHeaders().contains(HttpHeaders.SEC_FETCH_SITE)) {
    return FetchMetadataRuleResult.ALLOWED;
}
return FetchMetadataRuleResult.UNKNOWN;

A request with Sec-Fetch-Site: cross-site and an unparseable Dest then falls through to the remaining rules and gets rejected — fail-closed. The docs do describe today's behaviour ("absent, incomplete, and unrecognized"), so this is a deliberate call rather than an oversight, but I think it lands on the wrong side of the trade-off for a security default.

* excluding requests whose destination is {@code object} or {@code embed}.
*/
@Internal
@Singleton

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No enable/disable toggle, unlike the other five built-in rules.

Every sibling rule has a @Requires(property = ...) plus a matching accessor on FetchMetadataRulesConfiguration. This one has neither — and it's also missing the @Requires(classes = HttpRequest.class) guard the others carry.

That matters because this is the most permissive built-in rule: it allows cross-site GET navigations including frame/iframe destinations. It's exactly the rule an application hardening beyond the default policy would want to switch off, and right now there's no way to do that short of not using the module.

Suggest micronaut.security.fetch-metadata.rules.allow-simple-navigation (default true) for symmetry.

/**
* @return Whether to enable {@link SameSiteFetchMetadataRule} bean which allows requests whose Fetch Metadata identifies their initiator as the same site.
*/
boolean isAllowSameSite();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-on from the SimpleTopLevelNavigationAndIframingFetchMetadataRule comment: this interface exposes a toggle for five of the six built-in rules. Adding isAllowSimpleNavigation() here (plus the property and setter) would make the configuration surface complete and self-consistent.

Comment on lines +79 to +100
static boolean matchesOrigin(CorsOriginConfiguration configuration, String origin) {
if (configuration.getAllowedOriginsRegex()
.map(regex -> Pattern.matches(regex, origin))
.orElse(false)) {
return true;
}
List<String> allowedOrigins = configuration.getAllowedOrigins();
return !allowedOrigins.isEmpty() &&
((configuration.getAllowedOriginsRegex().isEmpty() &&
Objects.equals(allowedOrigins, CorsOriginConfiguration.ANY)) ||
allowedOrigins.contains(origin));
}

static boolean matchesMethod(CorsOriginConfiguration configuration, HttpRequest<?> request) {
HttpMethod requestMethod = request.getMethod();
HttpMethod methodToMatch = CorsUtil.isPreflightRequest(request)
? request.getHeaders().getFirst(ACCESS_CONTROL_REQUEST_METHOD, HttpMethod.class).orElse(requestMethod)
: requestMethod;
List<HttpMethod> allowedMethods = configuration.getAllowedMethods();
return Objects.equals(allowedMethods, CorsOriginConfiguration.ANY_METHOD) ||
allowedMethods.contains(methodToMatch);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates CorsFilter's private validation logic and will drift.

matchesOrigin and matchesMethod are near-verbatim reimplementations of micronaut-http-server's CorsFilter (5.1.12, lines 406-412, 423, 431-436) — right down to the allowedOriginsRegex.isEmpty() && isAny(...) subtlety, which is subtle enough that nobody will notice when it changes upstream.

Two consequences:

  1. It already diverges — CorsFilter additionally applies shouldDenyToPreventDriveByLocalhostAttack, which this copy omits, so the Fetch Metadata exemption is slightly more permissive than the CORS filter it claims to stay aligned with.
  2. Any future change to core's CORS semantics leaves this stale, silently and in the permissive direction.

Better options, roughly in order of preference: push a public helper into micronaut-core (CorsUtil.isOriginAllowed(config, origin) / isMethodAllowed(...)) and call it from here, or key the exemption off a request attribute that CorsFilter sets, rather than re-deriving the decision.

Minor, while you're in here: Pattern.matches(regex, origin) on line 81 recompiles the pattern on every cross-origin request.

* filter. For a preflight request, the requested method is read from the
* {@code Access-Control-Request-Method} header.</p>
*/
@Requires(classes = { HttpRequest.class, CorsOriginConfiguration.class, CrossOriginUtil.class })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CorsUtil is used on line 94 but isn't listed in @Requires(classes = ...). In practice all three classes ship in the same artifact so this can't actually blow up, but the list reads as if it's exhaustive — worth adding for consistency.

Comment on lines +92 to +96
if (LOG.isTraceEnabled()) {
LOG.trace("request {} {} rejected because no Fetch Metadata rule allowed it",
request.getMethod(), request.getPath());
}
return forbidden();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A 403 from a security filter is usually worth more than TRACE. As it stands an operator debugging a blocked client sees an unexplained 403 with nothing in the logs at default levels.

Suggest DEBUG for the rejection paths (keeping TRACE for the approvals), and including the actual Sec-Fetch-Site / -Mode / -Dest values in the message — that's the first thing anyone will want when triaging.

return forbidden();
}

private static MutableHttpResponse<Object> forbidden() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rejection is a bare 403 with no body and no extension point. Elsewhere in this repo rejections go through RejectionHandler, so applications can render a page, redirect, or add a header.

Even just a @DefaultImplementation-backed seam here would let applications customize the response without forking the filter.

}

/**
* Whether to enable {@link NoFetchMetadataRule} bean which allows requests without a complete set of Fetch Metadata headers.. Default value: {@value #DEFAULT_ALLOW_NO_FETCH_METADATA}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: headers.. — double period.

}
dependencies {
api(mn.micronaut.http)
api(mnValidation.validation)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment on FetchMetadataFilterConfigurationProperties: this api dependency exists solely for a @NotBlank that never gets evaluated. If the annotation goes, this can go with it.

|Enabled

|Simple navigation
|Allows `GET` requests with `Sec-Fetch-Mode: navigate`, except `object` and `embed` destinations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simple-navigation rule allows cross-site GET navigations with frame/iframe destinations — i.e. the default policy permits the application to be iframed cross-site. That matches the web.dev reference policy, so no complaint about the behaviour, but the table row ("except object and embed") makes it easy to read the opposite.

Worth one sentence noting that framing protection still requires CSP frame-ancestors or X-Frame-Options, so nobody assumes this module covers clickjacking.

Expose fetch metadata rule property constants

Move the built-in rule configuration prefix and property names to the public FetchMetadataRulesConfiguration interface. Update rule conditions and tests to reference the public constants instead of the package-private configuration properties implementation.
Log rejected requests at debug level when no Fetch Metadata rule allows them. Include the request destination, mode, and site values to make authorization failures easier to diagnose.
Javadoc typo: "…Fetch Metadata headers.." (double period) in setAllowNoFetchMetadata.
Allow requests as Fetch Metadata-unaware only when Sec-Fetch-Site is absent. Ensure requests with an incomplete or unrecognized Fetch Metadata value continue through the remaining rules, and update tests and documentation to describe the corrected behavior.
Add an allow-simple-navigation setting that conditionally enables the rule for cross-site GET navigations and iframes, preserving the existing enabled-by-default behavior.

Cover enabled and disabled configurations, verify that disabling the rule rejects cross-site navigation, and document the rule's framing implications.
Remove the @notblank constraint from the Fetch Metadata filter pattern and drop the now-unused validation dependency from the module.
Validate the fetch metadata filter pattern eagerly during application startup and fail configuration when it is empty or blank.

Add unit and context-startup coverage for invalid patterns, and document that the filter must be disabled explicitly instead of using an empty pattern.
@sonarqubecloud

sonarqubecloud Bot commented Sep 2, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
2 New Critical Issues (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement New feature or request

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

4 participants